-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ekump/adding-test-agent-to-trace-utils #516
Conversation
BenchmarksComparisonParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics.
CandidateCandidate benchmark detailsGroup 1
Warnings
BaselineBaseline benchmark detailsGroup 1
Warnings
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #516 +/- ##
==========================================
+ Coverage 70.21% 70.30% +0.08%
==========================================
Files 205 206 +1
Lines 27712 27806 +94
==========================================
+ Hits 19458 19549 +91
- Misses 8254 8257 +3
|
fb44a50
to
d4f05fa
Compare
464a373
to
f56fcee
Compare
f22875c
to
de0358b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I, for one, welcome our new docker running overlords...
de0358b
to
3a33805
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm impressed by how smooth it is to run test with docker within cargo 🤩
|
||
async fn get_base_uri_string(&self) -> String { | ||
let container_host = self.container.get_host().await.unwrap().to_string(); | ||
let container_port = self.container.get_host_port_ipv4(8126).await.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use TEST_AGENT_PORT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good catch.
trace-utils/tests/snapshots/compare_v04_trace_snapshot_test.json
Outdated
Show resolved
Hide resolved
7459183
to
f12fd00
Compare
ephemeral docker container. Also, add initial baseline snapshot test.
Only run tracing integration tests on ubuntu for now. Filter tracing integraiton tests out of regular tests and run specifically in its own step.
Co-authored-by: vianney <vianney.ruhlmann@datadoghq.com>
87734be
to
92c3fdd
Compare
What does this PR do?
Introduces the test agent to libdatadog to be used for integration tests in trace-utils.
The test agent runs inside a docker container that is spun up/down within a given test via testcontainers-rs.
The
DatadogTestAgent
struct has been created as a wrapper around the testcontainers implementation. It also provides some helper functions to make it easier to write tests that leverage the test agent.A single integration test has been added that performs a snapshot test via the test-agent for a trace.
Motivation
There is imminent work around trace_utils planned that could potentially modify trace payloads. These tests should provide some confidence for that work.
Additional Notes
The new tests naturally require docker. The README has been updated with instructions on how to skip these tests locally if you're not running docker. Alternatively, if people feel strongly we can invert this and default to not running these tests by default locally via a cargo feature.
The test will run on github CI only on Ubuntu for the time being.
How to test the change?
cargo nextest run